-
Notifications
You must be signed in to change notification settings - Fork 225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix key log encoding #682
Fix key log encoding #682
Conversation
|
||
type loggableRawKeyString string | ||
|
||
func (lk loggableRawKeyString) String() string { | ||
k := string(lk) | ||
|
||
if len(k) == 0 { | ||
return k | ||
} | ||
|
||
encStr := base32.RawStdEncoding.EncodeToString([]byte(k)) | ||
|
||
return encStr | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little unfortunate that this is here. It didn't seem worth it to make a logging subpackage, but I'm willing to budge here.
if err == nil { | ||
return newKey | ||
} | ||
return err.Error() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we always returned the key whenever there was an error (e.g. empty string, invalid path, etc.) now we return error messages.
@@ -128,7 +128,7 @@ func (dht *IpfsDHT) GetValue(ctx context.Context, key string, opts ...routing.Op | |||
if best == nil { | |||
return nil, routing.ErrNotFound | |||
} | |||
logger.Debugf("GetValue %v %v", key, best) | |||
logger.Debugf("GetValue %v %x", loggableRecordKeyString(key), best) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is, AFAICT, the only place we log record values. We now use hex encoding.
1958ff0
to
db2389a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return strings.ToLower(base32.RawStdEncoding.EncodeToString(k)) | ||
} | ||
|
||
func tryFormatLoggableRecordKey(k string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe tryParseAndFormat...
8f88727
to
b213268
Compare
Fixes #680
This is an attempt at normalizing how we log keys. Suggestions welcome, the main constraints are: